frontend/src/api/api-functions/images.ts (1)

17-24: Use endpoint constant and improve type safety.

Two issues:

1. The endpoint /images/search is hardcoded while fetchAllImages uses imagesEndpoints.getAllImages. Add a constant to apiEndpoints for consistency.
2. Return type Promise<any> loses type safety. Consider using Promise<APIResponse> to match fetchAllImages.

+// In apiEndpoints.ts, add:
+// searchImages: '/images/search',

-export const searchImages = async (query: string, tagged?: boolean): Promise<any> => {
+export const searchImages = async (query: string, tagged?: boolean): Promise<APIResponse> => {
   const params = new URLSearchParams({ query });
   if (tagged !== undefined) {
     params.append('tagged', tagged.toString());
   }
-  const response = await apiClient.get(`/images/search?${params.toString()}`);
+  const response = await apiClient.get<APIResponse>(
+    `${imagesEndpoints.searchImages}?${params.toString()}`
+  );
   return response.data;
 };

frontend/src/hooks/useImageSearch.ts (1)

4-10: Consider adding staleTime to reduce refetch frequency.

The hook works correctly, but since search results are unlikely to change rapidly, adding staleTime could improve UX by reducing unnecessary API calls during typing (even with debouncing in the UI).

 export const useImageSearch = (query: string, enabled: boolean = true) => {
   return usePictoQuery({
     queryKey: ['images', 'search', query],
     queryFn: () => searchImages(query),
     enabled: enabled && query.length > 0,
+    staleTime: 30000, // 30 seconds
   });
 };

backend/app/routes/images.py (2)

112-120: Remove redundant validation.

This manual check is unnecessary since Query(..., min_length=1) on line 98 already ensures the query is non-empty. FastAPI will return a 422 validation error before this code executes if the query is empty or whitespace-only (after strip).

     try:
-        if not query or not query.strip():
-            raise HTTPException(
-                status_code=status.HTTP_400_BAD_REQUEST,
-                detail=ErrorResponse(
-                    success=False,
-                    error="Validation Error",
-                    message="Search query cannot be empty",
-                ).model_dump(),
-            )
-
         images = db_search_images(query.strip(), tagged=tagged)

---

124-136: Metadata is being parsed twice.

db_search_images (in backend/app/database/images.py, line 468) already calls image_util_parse_metadata(metadata) and stores the parsed dict in images_dict[image_id]["metadata"]. Calling it again here on line 130 will pass an already-parsed dict through the function.

While image_util_parse_metadata handles dict input gracefully (returns it as-is), this is inefficient and potentially confusing. Either remove the call here or document the double-parse behavior.

         image_data = [
             ImageData(
                 id=image["id"],
                 path=image["path"],
                 folder_id=image["folder_id"],
                 thumbnailPath=image["thumbnailPath"],
-                metadata=image_util_parse_metadata(image["metadata"]),
+                metadata=image["metadata"],  # Already parsed in db_search_images
                 isTagged=image["isTagged"],
                 isFavourite=image.get("isFavourite", False),
                 tags=image["tags"],
             )
             for image in images
         ]

backend/app/database/images.py (1)

455-465: Unused location_name variable.

location_name is fetched from the query results (line 427, 464) but is never used in the function body. Either remove it from the SELECT clause or use it (e.g., for enriching search results).

-        for (
-            image_id,
-            path,
-            folder_id,
-            thumbnail_path,
-            metadata,
-            is_tagged,
-            is_favourite,
-            tag_name,
-            location_name,
-        ) in results:
+        for (
+            image_id,
+            path,
+            folder_id,
+            thumbnail_path,
+            metadata,
+            is_tagged,
+            is_favourite,
+            tag_name,
+            _location_name,  # Used in WHERE clause only
+        ) in results:

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

frontend/src/components/Navigation/Navbar/Navbar.tsx (3)

16-21: Debounced effect is coupled to searchState and can be simplified

Right now the effect depends on searchState.active / searchState.type but only uses them to gate clearSearch, which can:

• Dispatch startTextSearch twice per new query (once due to searchInput change, once after active/type flip).
• Auto‑clear a text search started elsewhere if searchInput is still '' when searchState flips to active.

If the intent is “Navbar owns text search and clearSearch is always safe when the input is empty”, you can decouple from searchState entirely and drop isSearchActive:

-  const isSearchActive = searchState.active && searchState.type === 'text';
+  // const isSearchActive = searchState.active && searchState.type === 'text';

   const dispatch = useDispatch();
   const [searchInput, setSearchInput] = useState('');

-  // Debounce search
-  useEffect(() => {
-    const timer = setTimeout(() => {
-      if (searchInput.trim().length > 0) {
-        dispatch(startTextSearch(searchInput.trim()));
-      } else if (searchInput.trim().length === 0 && isSearchActive) {
-        dispatch(clearSearch());
-      }
-    }, 500);
-
-    return () => clearTimeout(timer);
-
-    // use searchState
-  }, [searchInput, dispatch, searchState.active, searchState.type]);
+  // Debounce search based only on local input
+  useEffect(() => {
+    const trimmed = searchInput.trim();
+    const timer = setTimeout(() => {
+      if (trimmed) {
+        dispatch(startTextSearch(trimmed));
+      } else {
+        dispatch(clearSearch());
+      }
+    }, 500);
+    return () => clearTimeout(timer);
+  }, [searchInput, dispatch]);

This keeps behavior predictable (one dispatch per logical change, no hidden coupling to external text-search triggers).

Also applies to: 23-37

---

54-67: Consider whether Navbar should expose a way to clear the query image

The query image preview looks good, and the queryImage && ... guard keeps startsWith safe. However, with the inline close control removed, there’s no obvious way in the Navbar itself to “drop” the face/query image while staying on the same view.

If FaceSearchDialog is the only place to clear it, that’s fine; otherwise, consider re‑adding a small close/clear affordance on the thumbnail so users can quickly revert to a pure text search.

---

69-81: Wire up the Search icon button or make it non-interactive

The text input is already fully controlled and debounced, but the icon button is rendered as a <button> with no onClick, so it appears interactive yet does nothing. That’s confusing and not great for accessibility.

Either:

• Attach behavior (e.g., focus the input or immediately trigger a search), or
• Change it to a non-interactive element (e.g., <span> with appropriate classes and aria-hidden) so it’s clearly decorative.

Also applies to: 84-93

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

frontend/src/hooks/useImageSearch.ts (1)

4-9: Consider exposing the tagged filter parameter.

The searchImages API function supports an optional tagged parameter (see frontend/src/api/api-functions/images.ts lines 16-23), but this hook doesn't expose it. This limits search filtering capabilities.

♻️ Proposed enhancement to support tagged filter

-export const useImageSearch = (query: string, enabled: boolean = true) => {
+export const useImageSearch = (query: string, enabled: boolean = true, tagged?: boolean) => {
   return usePictoQuery({
-    queryKey: ['images', 'search', query],
-    queryFn: () => searchImages(query),
+    queryKey: ['images', 'search', query, tagged],
+    queryFn: () => searchImages(query, tagged),
     enabled: enabled && query.length > 0,
   });
 };

backend/app/database/images.py (2)

455-465: Unused variable location_name in result unpacking.

The location_name is unpacked from results (line 464) but never used in the image dictionary construction. Either use it or remove it from the SELECT and unpacking.

♻️ Either use location_name or remove it

If keeping location search but not the separate variable:

         for (
             image_id,
             path,
             folder_id,
             thumbnail_path,
             metadata,
             is_tagged,
             is_favourite,
             tag_name,
-            location_name,
+            _,  # location_name not needed separately
         ) in results:

---

414-414: Performance consideration: Leading wildcard prevents index usage.

The search pattern f"%{query}%" with leading wildcard will cause full table scans. For large image collections, this could be slow.

Consider documenting this limitation or exploring alternatives like FTS (Full-Text Search) for future scalability, as mentioned in the PR objectives.

frontend/src/components/Navigation/Navbar/Navbar.tsx (1)

21-34: Debounce effect looks correct but consider clearing search on unmount.

The debounce implementation is clean. However, if the component unmounts while a search is active, the search state remains in Redux. Consider clearing search on unmount if that's the desired UX.

♻️ Optional: Clear search on unmount

   useEffect(() => {
     const trimmed = searchInput.trim();

     const timer = setTimeout(() => {
       if (trimmed) {
         dispatch(startTextSearch(trimmed));
       } else {
         dispatch(clearSearch());
       }
     }, 500);

-    return () => clearTimeout(timer);
+    return () => {
+      clearTimeout(timer);
+    };
   }, [searchInput, dispatch]);
+
+  // Optional: Clear search when navigating away
+  useEffect(() => {
+    return () => {
+      dispatch(clearSearch());
+    };
+  }, [dispatch]);

frontend/src/pages/Home/Home.tsx (1)

80-83: Add Array.isArray validation for normal fetch data.

The text search path (lines 70-74) validates that the data is an array, but the normal fetch path doesn't have the same validation.

♻️ Proposed fix for consistency

     // No search → normal image fetch
     if (!searchState.active && isSuccess) {
       const images = (data?.data || []) as Image[];
+      if (!Array.isArray(images)) {
+        console.error("Invalid data format");
+        return;
+      }
       dispatch(setImages(images));
     }

backend/app/database/images.py (1)

414-443: Upstream validation already prevents empty queries at the route level.

The search_images endpoint in backend/app/routes/images.py validates the query parameter with Query(..., min_length=1) and explicitly checks if not query or not query.strip() before calling db_search_images(). This guard prevents "%%" from reaching the database function.

However, adding input validation at the database layer is still a reasonable defensive measure if the function might be called from other code paths outside the route. If the function is intended as an internal database utility only, the existing route-level validation is sufficient.